Skip to content

Add Cards/Files/Definitions metadata to workspace favorites - #5677

Open
cmgardella wants to merge 12 commits into
mainfrom
workspace-chooser-favorites-metadata
Open

Add Cards/Files/Definitions metadata to workspace favorites#5677
cmgardella wants to merge 12 commits into
mainfrom
workspace-chooser-favorites-metadata

Conversation

@cmgardella

Copy link
Copy Markdown
Contributor

Summary

  • Favorite workspace tiles now show real Cards/Files/Definitions counts pulled from the realm index, replacing the previous card-count/recent-activity stats and the collaborator avatar stack.
  • The "New Workspace" tile now renders first in Your Workspaces instead of last, and workspaces/catalogs sort newest-first.
  • Added getFileCount/getDefinitionCount queries to Realm alongside the existing getCardCount, and threaded the new RealmInfo fields through.
  • Added a functional data-menu-item-id attribute to the shared Menu component so per-item CSS no longer has to select on a test-only data-test-* attribute.

Test plan

  • eslint and ember-template-lint pass on all changed files
  • Manually verified in a local dev environment: favorite tiles render varied Cards/Files/Definitions counts reflecting real realm content, the New Workspace tile appears first, and the collaborator avatars are gone

…New Workspace tile

Favorite tiles now show real Cards/Files/Definitions counts pulled from the
realm index instead of card-count/recent-activity stats, and the collaborator
avatar stack is removed. The "New Workspace" tile now sits first in Your
Workspaces instead of last.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@cmgardella
cmgardella requested a review from lukemelia August 3, 2026 18:43
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 45m 1s ⏱️
3 490 tests 3 472 ✅ 12 💤 4 ❌ 2 🔥
3 509 runs  3 489 ✅ 12 💤 6 ❌ 2 🔥

Results for commit 09754f9.

For more details on these errors, see this check.

Realm Server Test Results

    1 files      1 suites   13m 10s ⏱️
2 077 tests 2 076 ✅ 0 💤 1 ❌
2 156 runs  2 155 ✅ 0 💤 1 ❌

Results for commit 09754f9.

For more details on these errors, see this check.

lukemelia and others added 6 commits August 3, 2026 19:18
Cards/Files/Definitions counts and the realm's created/updated timestamps
now come from `Realm#getDetailedRealmInfo`, used by both `/_info` and the
realm server's batch `/_federated-info` — the latter is what the host's
workspace chooser actually reads.

They are deliberately not part of `parseRealmInfo`/`getRealmInfo`, whose
result is embedded in every card response's `meta.realmInfo` and hashed into
the card+json ETag: values that move on an ordinary realm write would
invalidate every card's cached representation in the realm whenever one card
changed, and cost extra queries on every card request.

Counting fixes:

- Count per distinct url rather than per row. A card instance is indexed as
  both an `instance` row and a `file` row at the same url, so counting rows
  put every card into the file count too.
- Drop the `generation = current_generation` predicate. That column is a
  last-touched watermark that an incremental index only bumps on the rows it
  rewrote, so pinning it counted the files touched by the most recent index
  pass rather than the realm's contents. Deletions are tombstoned via
  `is_deleted`, matching how the query engine scopes a live search.
- Use adapter-portable SQL. `count(*)::int` is Postgres-only and threw on the
  sqlite adapter the host tests use.

Read realm_metadata and realm_registry independently instead of joining them.
Keying the metadata read off realm_registry dropped showAsCatalog/publishable
for any realm with a metadata row but no registry row.

Drop recentActivityCount and collaboratorUsernames: nothing renders them, and
the collaborator list exposed every matrix user with realm access to any
realm reader — a wider audience than the owner-gated `/_permissions` route.

Workspace chooser:

- Keep the initial keyboard selection on a workspace. The New Workspace tile
  now renders first, and the selected tile takes focus, so starting there made
  the first Enter create a workspace instead of opening one.
- Restore the `.is-selected` ring, so keyboard-selected tiles stay visible.
- Give the favorite star a tooltip, sharing one getter with its aria-label.
- Collapse the three repeated stat blocks into one `tileStats` loop.

Tests: rewrite the keyboard-navigation tests for the new tile order, and add
coverage for the metadata row, tile ordering, catalog sort, the menu footer,
the star tooltip, the date formatters, and the index counts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
/_info runs an aggregate over every index row in the realm, and the realm
server's /_catalog-realms fans out to one _info per catalog realm on top of
the host's own calls. Uncached, that took per-realm _info from a ~44ms median
to ~200ms (max 762ms) in the matrix suite.

Cached separately from #cachedRealmInfo rather than folded into it, because
that object is hashed into the card+json ETag and these values move on every
realm write. Dropped by the same paths that drop #cachedRealmInfo, so the
counts still refresh on every index swap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
/_catalog-realms issues one _info per publicly-readable realm, drops any
realm whose response isn't a 200, and caches the resulting list for the life
of the server process. Putting per-request index aggregation behind that cold
fan-out risked the catalog list for no benefit: the workspace chooser reads
its tile metadata from /_federated-info, which still serves the detailed
variant.

/_info is now byte-identical to main again. The count assertions move to
Realm#getDetailedRealmInfo directly, and a new test pins the contract that
/_info omits the extras.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only a favorited workspace tile renders the Cards / Files / Definitions row,
but the counts were riding along on /_federated-info, which the host loads for
every realm at boot. The counts are the expensive half — an aggregate over
every index row in a realm — while the realm timestamps beside them are a
single indexed row lookup, so the two are split:

- /_federated-info keeps the timestamps. Needed more widely than the counts:
  createdAt orders the catalog list, and both feed the per-tile options menu.
- /_federated-index-counts is new, behind the same multiRealmAuthorization
  contract, and returns counts only for the realms it is asked about.

The host requests counts for favoriteRealmIdentifiers alone, from a modifier on
the Favorites list so it runs after render and re-runs when the set changes.
RealmService.loadIndexCounts is fire-and-forget and skips realms already loaded
or in flight, so the dashboard never waits on it; counts land in a tracked map
keyed by realm URL, separate from the realm info because they arrive on their
own schedule.

The stats row is now rendered unconditionally on favorite tiles with its height
reserved, so the numbers fill existing space rather than growing the tile and
shifting its centered name.

Counts stay memoized per index generation on the realm and are dropped by the
same invalidation paths as the realm info, so a re-render costs nothing and a
write is still reflected after the swap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
If the realm fixture build fails, the realms are never assigned and an
unguarded teardown throws on unsubscribe before closeServer runs — leaking the
bound port so every later test in the process reports EADDRINUSE instead of the
original failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The realm server dropped its memoized counts on index swap, but the host kept
its copy for the session — so a favorited tile showed numbers from whenever the
chooser first asked. The only existing re-index refresh is deliberately narrow
(RealmConfig-card invalidations, for renames), and widening that would clobber
client-managed publish state.

Counts have no such hazard, so any completed index now marks them stale.
Marked, not refetched: the displayed numbers stay on screen so a write doesn't
blank the stats row, and a realm nobody is looking at costs nothing. The
workspace chooser's loader takes a revision argument that changes on
invalidation, which is what re-triggers the fetch on its next render — keyed
only on which realms are favorited, it would never re-run when the answer for
those realms changed.

Also correct the endpoint test's expected card count: a realm's own RealmConfig
card at realm.json is an instance, so the seeded realm holds two cards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates workspace favorites to display real realm-index-derived metadata (Cards/Files/Definitions counts plus lifecycle timestamps), adjusts workspace/catalog ordering in the chooser, and introduces a shared menu-item identifier hook for styling without relying on test-only attributes.

Changes:

  • Added realm lifecycle timestamps (createdAt, updatedAt) to the boot-time federated realm info payload, while keeping index-aggregate counts on a separate, lazy path.
  • Introduced a new realm-server endpoint /_federated-index-counts and host-side caching/invalidation to load tile counts only for favorited realms.
  • Updated workspace chooser UI/behavior (favorite tile layout + tooltips, menu footer timestamps, new workspace tile ordering, and navigation expectations) and added data-menu-item-id to menu items.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/runtime-common/realm.ts Adds lifecycle timestamps plumbing and index-count aggregation + memoized APIs (getDetailedRealmInfo, getIndexCounts).
packages/runtime-common/index.ts Exports RealmIndexCounts.
packages/runtime-common/helpers/const.ts Adds helpers for asserting/stripping federated-only realm-info extras in tests.
packages/realm-server/routes.ts Registers the new /_federated-index-counts route.
packages/realm-server/handlers/handle-realm-info.ts Switches federated info to use getDetailedRealmInfo() (timestamps included).
packages/realm-server/handlers/handle-realm-index-counts.ts Implements the federated index-counts endpoint backed by Realm#getIndexCounts().
packages/realm-server/tests/server-endpoints/index-counts-test.ts Adds endpoint-level coverage for counts, auth behavior, and staleness after reindex.
packages/realm-server/tests/server-endpoints/info-test.ts Updates federated-info tests to assert presence/parseability of timestamp extras.
packages/realm-server/tests/server-endpoints/user-and-catalog-test.ts Updates catalog-realms test expectations to match plain /_info payload shape.
packages/realm-server/tests/realm-endpoints/info-test.ts Adds realm-level assertions for counts bucketing and timestamps; guards that /_info stays lean.
packages/realm-server/tests/index.ts Registers the new server-endpoints test module.
packages/realm-server/tests/helpers/index.ts Adds shared assertion helpers for realm-info extras and index-counts payloads.
packages/host/app/services/realm.ts Adds tracked, lazy index-count loading/caching + invalidation on index events; threads timestamps into default realm info objects.
packages/host/app/services/realm-server.ts Adds fetchRealmIndexCounts() client method for the new endpoint.
packages/host/app/components/operator-mode/workspace-chooser/index.gts Adds createdAt-based sorting, lazy count-loading modifier, responsive favorite tile sizing, and updated selection/nav model.
packages/host/app/components/operator-mode/workspace-chooser/workspace.gts Renders enlarged favorite tiles with stats/tooltips, adds timestamps footer in menu, and updates styling/hover behavior.
packages/host/tests/acceptance/workspace-chooser-test.gts Expands acceptance coverage for tooltips, favorite-tile stats behavior, tile ordering, and keyboard navigation changes.
packages/host/tests/unit/workspace-timestamp-labels-test.ts Adds unit coverage for relative-time formatting helpers.
packages/host/app/components/operator-mode/submode-layout.gts Updates top-bar center layout and avatar border token usage.
packages/boxel-ui/addon/src/components/menu/index.gts Adds data-menu-item-id attribute to menu item content for stable per-item styling hooks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/host/tests/acceptance/workspace-chooser-test.gts Outdated
…omment

The divider's hardcoded #e8e8e8 is exactly --boxel-200, so this swap is
byte-identical in output while going through the palette like the rest of the
chrome.

The favorite-tile metadata module's comment still described the counts as coming
from each realm's /_info, which stopped being true when they moved to
/_federated-index-counts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lukemelia
lukemelia requested review from a team and burieberry and removed request for lukemelia August 4, 2026 17:00

@burieberry burieberry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Findings 1 and 2 are verified against the source (2 by code-reading, not reproduced); findings 3–11 are not verified at all. The unverified ones come from a code-reading pass — each cites a concrete call site, but nothing was reproduced and no test was written against any of them. Treat them as leads to check, not confirmed defects. The CSS/layout ones especially want a browser rather than a code read.

Worth checking before merge

1. realm_registry.updated_at never moves for source realmspackages/runtime-common/realm.ts:6892

getRegistryTimestamps reads created_at, updated_at FROM realm_registry, but only two code paths ever write realm_registry.updated_at, and both exclude source rows: realm-registry-writes.ts:77 (… DO UPDATE SET last_published_at = …, updated_at = now() WHERE realm_registry.kind = 'published') and realm-registry-backfill.ts:153 (same shape, WHERE … kind = 'bootstrap'). insertSourceRealmInRegistry inserts kind = 'source' with ON CONFLICT (url) DO NOTHING and never sets it, and the schema (1776960113000_create-realm-registry.js:74) has no trigger — just DEFAULT now().

So for every source workspace updated_at === created_at permanently, and the menu footer shows a fixed "Updated 3 months ago" regardless of how much the user writes. The comment at realm.ts:7222 — "updated_at — which moves on any ordinary realm write" — is inaccurate, and since the ETag argument is built on it, that reasoning needs revisiting too.

(The other updated_at = now() writers in the tree — handle-publish-realm.ts:539, create-realm.ts:169, realm-metadata-queries.ts:24,35 — all target realm_metadata, a different table.)

2. measureWorkspaceGrid writes tracked state already consumed in the same renderpackages/host/app/components/operator-mode/workspace-chooser/index.gts:306

The modifier's body ends in a bare synchronous measure(), which writes workspaceGridWidth/workspaceTileWidth/workspaceTileGap. But it's installed on the Your Workspaces list, which renders after the Favorites block — and Favorites passes @enlargedWidth={{this.favoritesTileWidthPx}}, whose getter chain reads workspaceTileWidth (the if (!this.workspaceTileWidth) return null early-return consumes the tag too). So for any user who already has a favorite when the chooser opens, the read happens first and the modifier writes into the same render transaction: Ember's backtracking-rerender assertion in dev builds. This is the same hazard documented for the onFocusIn path above — "Writing selectedIndex there trips Ember's backtracking-rerender assertion."

No test catches it: openChooserWithFavorite assigns workspaceFavorites after visitOperatorMode, because (per its own comment) login resets the matrix service and drops a pre-visit assignment. By then the modifier is installed, and with no arguments it never re-invokes — so measure() never runs in a transaction that reads favoritesTileWidthPx. That awkwardness is also why I couldn't build a repro harness; I verified the ordering by reading, not by observing the assertion. Manual check: favorite a workspace, reopen the chooser in a dev build, watch the console.

Scheduling the first measure() via next/scheduleOnce('afterRender') fixes it and costs one extra frame on open.

Robustness

3. A failed counts fetch never retriespackages/host/app/services/realm.ts:937

The catch logs and leaves the map untouched, so nothing trackFavoriteCounts depends on changes (same favoriteCountsKey, same indexCountsRevision) and the modifier doesn't re-run. One transient 500/offline blip — or a realm the server omitted because getIndexCounts() threw — would leave every favorite tile with a blank stats row for the session. The comment says "a later attempt retries"; I couldn't find the trigger.

4. getIndexCounts caches failures and doesn't dedupe concurrent callerspackages/runtime-common/realm.ts:7247

A queryIndexCounts() failure returns an all-null object, which is truthy, so it's cached until the next index swap — one DB hiccup means no stats until that realm re-indexes. And since only the resolved value is memoized rather than the promise, N concurrent /_federated-index-counts requests each run the full per-url aggregate. Caching the promise and skipping the null-triple would cover both.

5. Stale-marking races the first loadpackages/host/app/services/realm.ts:893

markIndexCountsStale early-returns when indexCountsByRealm has no entry. If a realm finishes indexing while its first counts request is in flight, the mark is dropped and the in-flight response (computed pre-swap) is then stored as fresh. Marking inFlightIndexCounts members stale, or bumping the epoch unconditionally, would close it.

6. Only the batch route carries the new timestampspackages/host/app/services/realm.ts:426

fetchInfoFromServer hits per-realm /_info, which this PR deliberately leaves on plain getRealmInfo(). Any realm loading through fetchInfo/refreshInfo rather than the boot-time /_federated-info batch — a workspace created mid-session, say — would have createdAt/updatedAt undefined, suppressing its footer and (for catalogs) sorting it to the bottom of sortByCreatedAtDesc. A reload masks it, which is what makes it easy to miss.

7. timestamp without time zone reinterpreted in the process TZpackages/runtime-common/realm.ts:623

created_at/updated_at are timestamp (no tz) defaulted from now(). pg parses that as local time, so toISOStringOrNull shifts by the process's UTC offset whenever the realm-server's TZ differs from the DB session's. On a non-UTC box that reads as "Created 8 hrs ago", or lands in the future and clamps to "Updated just now". timestamptz, or an explicit AT TIME ZONE 'UTC' in the SELECT, would be sturdier.

UI / accessibility — needs a browser to confirm

8. Keyboard focus on the Options button is invisiblepackages/host/app/components/operator-mode/workspace-chooser/workspace.gts:665

.tile-menu-btn gains opacity: 0, and the old :focus-within rule was replaced by :has([aria-expanded='true']). A keyboard user tabbing through a tile would land on a fully transparent focused button with no visible indication until they open the dropdown. Re-adding :focus-within { opacity: 1 } alongside the new rule should fix it.

9. .tile-status-bar may swallow tile clickspackages/host/app/components/operator-mode/workspace-chooser/workspace.gts:708

It's a sibling of the ItemContainer button at position: absolute; z-index: 3 with default pointer-events (which its tooltips need). On an enlarged favorite tile that would make the hosted/visibility pill an inert strip across the top-left — clicking it does nothing instead of opening the workspace.

10. defaultSelectedIndex lands on "New Workspace" in the empty statepackages/host/app/components/operator-mode/workspace-chooser/index.gts:382

With no favorites and no user workspaces (new user, or everything archived/filtered), both branches fall through to return 0, which is addWorkspaceNavIndex, and focusWhenSelected focuses it — so the first Enter opens the create-workspace modal, the hazard the surrounding comment warns about. Falling back to catalogNavBase when catalogs exist would avoid it.

11. The top-bar center is now out of flowpackages/host/app/components/operator-mode/submode-layout.gts:678

Swapping flex: 1 for position: absolute; left: 50%; transform: translate(-50%, -50%) drops the flex guarantee that the center group can't collide with the left (workspace/search) and right (profile) groups. At narrow container widths it would overlap them and, as a normal pointer-events element higher in the stack, intercept clicks on the controls underneath.

🤖 Reviewed by Claude (Opus 5)

@burieberry

Copy link
Copy Markdown
Contributor

⚠️ Findings 1 and 2 are verified against the source (2 by code-reading, not reproduced); findings 3–11 are not verified at all. The unverified ones come from a code-reading pass — each cites a concrete call site, but nothing was reproduced and no test was written against any of them. Treat them as leads to check, not confirmed defects. The CSS/layout ones especially want a browser rather than a code read.

Worth checking before merge

1. realm_registry.updated_at never moves for source realmspackages/runtime-common/realm.ts:6892

getRegistryTimestamps reads created_at, updated_at FROM realm_registry, but only two code paths ever write realm_registry.updated_at, and both exclude source rows: realm-registry-writes.ts:77 (… DO UPDATE SET last_published_at = …, updated_at = now() WHERE realm_registry.kind = 'published') and realm-registry-backfill.ts:153 (same shape, WHERE … kind = 'bootstrap'). insertSourceRealmInRegistry inserts kind = 'source' with ON CONFLICT (url) DO NOTHING and never sets it, and the schema (1776960113000_create-realm-registry.js:74) has no trigger — just DEFAULT now().

So for every source workspace updated_at === created_at permanently, and the menu footer shows a fixed "Updated 3 months ago" regardless of how much the user writes. The comment at realm.ts:7222 — "updated_at — which moves on any ordinary realm write" — is inaccurate, and since the ETag argument is built on it, that reasoning needs revisiting too.

(The other updated_at = now() writers in the tree — handle-publish-realm.ts:539, create-realm.ts:169, realm-metadata-queries.ts:24,35 — all target realm_metadata, a different table.)

2. measureWorkspaceGrid writes tracked state already consumed in the same renderpackages/host/app/components/operator-mode/workspace-chooser/index.gts:306

The modifier's body ends in a bare synchronous measure(), which writes workspaceGridWidth/workspaceTileWidth/workspaceTileGap. But it's installed on the Your Workspaces list, which renders after the Favorites block — and Favorites passes @enlargedWidth={{this.favoritesTileWidthPx}}, whose getter chain reads workspaceTileWidth (the if (!this.workspaceTileWidth) return null early-return consumes the tag too). So for any user who already has a favorite when the chooser opens, the read happens first and the modifier writes into the same render transaction: Ember's backtracking-rerender assertion in dev builds. This is the same hazard documented for the onFocusIn path above — "Writing selectedIndex there trips Ember's backtracking-rerender assertion."

No test catches it: openChooserWithFavorite assigns workspaceFavorites after visitOperatorMode, because (per its own comment) login resets the matrix service and drops a pre-visit assignment. By then the modifier is installed, and with no arguments it never re-invokes — so measure() never runs in a transaction that reads favoritesTileWidthPx. That awkwardness is also why I couldn't build a repro harness; I verified the ordering by reading, not by observing the assertion. Manual check: favorite a workspace, reopen the chooser in a dev build, watch the console.

Scheduling the first measure() via next/scheduleOnce('afterRender') fixes it and costs one extra frame on open.

Robustness

3. A failed counts fetch never retriespackages/host/app/services/realm.ts:937

The catch logs and leaves the map untouched, so nothing trackFavoriteCounts depends on changes (same favoriteCountsKey, same indexCountsRevision) and the modifier doesn't re-run. One transient 500/offline blip — or a realm the server omitted because getIndexCounts() threw — would leave every favorite tile with a blank stats row for the session. The comment says "a later attempt retries"; I couldn't find the trigger.

4. getIndexCounts caches failures and doesn't dedupe concurrent callerspackages/runtime-common/realm.ts:7247

A queryIndexCounts() failure returns an all-null object, which is truthy, so it's cached until the next index swap — one DB hiccup means no stats until that realm re-indexes. And since only the resolved value is memoized rather than the promise, N concurrent /_federated-index-counts requests each run the full per-url aggregate. Caching the promise and skipping the null-triple would cover both.

5. Stale-marking races the first loadpackages/host/app/services/realm.ts:893

markIndexCountsStale early-returns when indexCountsByRealm has no entry. If a realm finishes indexing while its first counts request is in flight, the mark is dropped and the in-flight response (computed pre-swap) is then stored as fresh. Marking inFlightIndexCounts members stale, or bumping the epoch unconditionally, would close it.

6. Only the batch route carries the new timestampspackages/host/app/services/realm.ts:426

fetchInfoFromServer hits per-realm /_info, which this PR deliberately leaves on plain getRealmInfo(). Any realm loading through fetchInfo/refreshInfo rather than the boot-time /_federated-info batch — a workspace created mid-session, say — would have createdAt/updatedAt undefined, suppressing its footer and (for catalogs) sorting it to the bottom of sortByCreatedAtDesc. A reload masks it, which is what makes it easy to miss.

7. timestamp without time zone reinterpreted in the process TZpackages/runtime-common/realm.ts:623

created_at/updated_at are timestamp (no tz) defaulted from now(). pg parses that as local time, so toISOStringOrNull shifts by the process's UTC offset whenever the realm-server's TZ differs from the DB session's. On a non-UTC box that reads as "Created 8 hrs ago", or lands in the future and clamps to "Updated just now". timestamptz, or an explicit AT TIME ZONE 'UTC' in the SELECT, would be sturdier.

UI / accessibility — needs a browser to confirm

8. Keyboard focus on the Options button is invisiblepackages/host/app/components/operator-mode/workspace-chooser/workspace.gts:665

.tile-menu-btn gains opacity: 0, and the old :focus-within rule was replaced by :has([aria-expanded='true']). A keyboard user tabbing through a tile would land on a fully transparent focused button with no visible indication until they open the dropdown. Re-adding :focus-within { opacity: 1 } alongside the new rule should fix it.

9. .tile-status-bar may swallow tile clickspackages/host/app/components/operator-mode/workspace-chooser/workspace.gts:708

It's a sibling of the ItemContainer button at position: absolute; z-index: 3 with default pointer-events (which its tooltips need). On an enlarged favorite tile that would make the hosted/visibility pill an inert strip across the top-left — clicking it does nothing instead of opening the workspace.

10. defaultSelectedIndex lands on "New Workspace" in the empty statepackages/host/app/components/operator-mode/workspace-chooser/index.gts:382

With no favorites and no user workspaces (new user, or everything archived/filtered), both branches fall through to return 0, which is addWorkspaceNavIndex, and focusWhenSelected focuses it — so the first Enter opens the create-workspace modal, the hazard the surrounding comment warns about. Falling back to catalogNavBase when catalogs exist would avoid it.

11. The top-bar center is now out of flowpackages/host/app/components/operator-mode/submode-layout.gts:678

Swapping flex: 1 for position: absolute; left: 50%; transform: translate(-50%, -50%) drops the flex guarantee that the center group can't collide with the left (workspace/search) and right (profile) groups. At narrow container widths it would overlap them and, as a normal pointer-events element higher in the stack, intercept clicks on the controls underneath.

🤖 Reviewed by Claude (Opus 5)

8 is confirmed. 11 is correct that there's overlap:

overlap

@burieberry

Copy link
Copy Markdown
Contributor

Follow-up on finding 2, on the shape of the fix — measureWorkspaceGrid can lose two of its three measurement triggers, which turns that finding into a deletion rather than an addition.

The window.addEventListener('resize', measure) is redundant. Anything that changes the viewport width also changes el.clientWidth, so the ResizeObserver already covers every case the window listener does. The reverse isn't true — the window listener misses a scrollbar appearing when the workspace list grows past the viewport height (narrows el, no resize event; adding a workspace or WorkspaceLoadingIndicator rendering can both do it), any surrounding panel changing width, and zoom/font-size changes, which are inconsistent about firing resize. The observer is the one worth keeping.

The trailing synchronous measure() is also redundant, and it's what trips the assertion. Per spec, observe() delivers an initial callback with the element's current size, and that callback lands after the render transaction commits rather than inside it. So dropping the bare measure() gets you the first measurement anyway and fixes finding 2 without needing an explicit next/scheduleOnce('afterRender') wrapper:

let ro;
if (typeof ResizeObserver !== 'undefined') {
  // observe() delivers an initial callback with the current size, so this
  // covers the first measurement too — and it lands after the render
  // transaction commits, which keeps the tracked writes out of it.
  ro = new ResizeObserver(measure);
  ro.observe(el);
}
return () => ro?.disconnect();

I checked the ResizeObserver-absent branch before suggesting this, since it's the obvious objection — and it degrades gracefully on its own: tilesPerRow returns 3, favoritesSlotWidth returns null, favoritesTileWidthPx returns undefined, and the tile falls back to its static CSS width. "Never measured" is already a supported state, so no else branch is needed to stay correct. (Which also raises the question of whether the typeof guard is needed at all — ResizeObserver has been baseline for years — but that's a judgement call about which render paths you expect this to run in.)

Preferring this over wrapping measure() in next/afterRender for two reasons: it leaves one measurement path instead of two doing identical work on open, and RO's before-paint delivery is strictly earlier than a next() macrotask. Neither approach can promise no single-frame flash of the static width, since the tracked write still routes through a backburner flush either way.

One caveat on provenance: the initial-callback behavior is from the spec, not something I re-confirmed in a browser here. The getter fallbacks above I did verify against the diff.

🤖 Reviewed by Claude (Opus 5)

ylm and others added 2 commits August 5, 2026 14:19
…chooser fixes

Correctness and robustness follow-ups on the favorites-metadata work:

- realm_registry.updated_at now advances on every write to a source realm
  (touched from the incremental-index invalidation hook), so the workspace
  chooser's "Updated" footer reflects real activity instead of staying pinned
  to created_at. created_at/updated_at widen to timestamptz so the stored
  instant is unambiguous regardless of the realm-server's TZ.
- Server getIndexCounts memoizes the in-flight promise (concurrent callers
  share one aggregate) and no longer caches an all-null failure result.
- Host index-count loading retries realms left stale by a failed or omitted
  fetch on a delayed (non-hot-looping) schedule, and no longer drops a stale
  mark that races a first load. A mid-session workspace also picks up its
  lifecycle timestamps via the federated batch rather than the lean /_info.
- Chooser UI: ResizeObserver's post-commit initial callback replaces the
  synchronous measure() that tripped the backtracking-rerender assertion;
  the empty state no longer lands selection on "New Workspace"; the Options
  button reveals on :focus-within for keyboard users; the status pill lets
  tile clicks through; and the top bar's center stays in flow so it can't
  overlap or intercept the workspace/profile controls.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every host CI shard failed in __shard_warmup__: the boot-time info fetch
now asks the federated /_info batch for lifecycle timestamps, and when it
runs before matrix-service start() has handed the client to
RealmServerService, loginTask throws "Cannot login to realm server
without matrix client". That throw happened before the try/finally that
clears `loggingIn`, so the rejected task instance stayed cached and
start()'s own login() call — made after setClient — awaited the stale
rejection and surfaced it as an uncaught global error, aborting the
shard.

Two changes:
- loginTask clears `loggingIn` on every exit, and distinguishes the
  no-client precondition failure (propagated) from an auth failure
  (logged, token cleared) so a later attempt with a client performs
  fresh.
- prefetchRealmInfos no-ops until the matrix client exists: the
  federated batch needs a realm-server session, so pre-login callers
  (app boot, anonymous access) fall through to their lean per-realm
  fetches, and the post-login boot re-runs the batch for every
  available realm.

Also drop the unused eslint-disable directive that failed lint in the
realm-registry timestamps migration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lukemelia

Copy link
Copy Markdown
Contributor

[Claude Code 🤖] All eleven findings are addressed on the branch — findings 1–11 in e80d84e, with a follow-up in eaf8701 after finding 6's fix broke CI (details under 6). Per finding:

1. realm_registry.updated_at never moves for source realms — Confirmed and fixed in e80d84e. touchSourceRealmUpdatedAt() runs from both incremental-index invalidation hooks, so any write that indexes advances the timestamp — once per write batch, scoped to kind = 'source' (published rows keep publish-time semantics via last_published_at, bootstrap rows never change), and best-effort so a failed touch can't fail the index that triggered it. The ETag-rationale comment now names this mechanism instead of asserting a behavior that didn't exist.

2. Backtracking-rerender in measureWorkspaceGrid — Fixed in e80d84e, taking the deletion shape from your follow-up comment; replied there.

3. Failed counts fetch never retries — Fixed in e80d84e. A failed or partial response marks every requested realm stale and schedules one delayed wake that bumps the revision consumers already track, so their loaders refetch — a slow poll while the failure persists, self-stopping on success. The "a later attempt retries" comment now has an actual trigger behind it.

4. getIndexCounts caches failures and doesn't dedupe concurrent callers — Fixed in e80d84e in the shape you suggested: the in-flight promise is memoized so concurrent /_federated-index-counts callers share one aggregate, the all-null failure triple is never cached (the next request retries), and an index swap drops the in-flight promise too so an overlapping request can't repopulate the cache with pre-swap counts.

5. Stale-mark racing the first load — Fixed in e80d84e. markIndexCountsStale also marks realms in inFlightIndexCounts, and loadIndexCounts claims staleness when the fetch starts and re-checks it in finally, so a mid-flight index swap forces a refetch rather than letting the pre-swap response be stored as fresh.

6. Timestamps only on the batch route — Fixed in e80d84e: fetchInfoTask asks the federated batch for the single realm before falling back to the lean per-realm /_info, so a workspace created mid-session picks up its lifecycle timestamps. That call is also what turned every host CI shard red: at shard warm-boot it runs before matrix start() hands the client to RealmServerService, loginTask threw its no-client error before the finally that clears loggingIn, and every later login() — including the one start() makes right after setClient — awaited the stale rejected task instance and surfaced the error as an uncaught global failure. eaf8701 fixes both layers: loginTask now clears loggingIn on every exit (a pre-client attempt can no longer poison later logins), and prefetchRealmInfos no-ops until the matrix client exists — pre-login callers fall through to the lean per-realm fetch, while the post-login boot re-runs the batch for every available realm, so the mid-session-workspace case this finding is about (which only arises logged-in) still gets its timestamps.

7. timestamp without time zone reinterpreted in the process TZ — Confirmed and fixed in e80d84e. A migration widens created_at/updated_at to timestamptz, reinterpreting the existing wall-clock values as UTC (they were written by now() against a UTC session). It sits in the additive phase: the previously deployed revision neither reads these columns nor inserts anything now() can't satisfy in either type.

8. Invisible keyboard focus on the Options button — Thanks for confirming it in a browser; fixed in e80d84e with the :focus-within reveal you suggested, alongside the :has([aria-expanded='true']) open-state rule.

9. .tile-status-bar swallowing tile clicks — Fixed in e80d84e. The bar is pointer-events: none so clicks fall through to the tile's open-workspace button, and the two tooltip triggers re-enable pointer-events: auto on themselves so hovering the hosted/visibility icons still surfaces their tooltips.

10. Empty state lands selection on "New Workspace" — Fixed in e80d84e with the fallback you suggested: with no favorites and no user workspaces, selection prefers the first catalog when one renders, and only lands on index 0 when there is genuinely nothing else to land on.

11. Top-bar center out of flow — Confirmed by your screenshot; fixed in e80d84e. The center group is a flex: 1 flex child again, so the middle track is reserved between the workspace and profile controls — the portaled expanded-card-header pill can neither slide under them at narrow widths nor intercept their clicks, and it stays centered within the track via its own max-width/margin: auto.

CI is re-running on eaf8701.

@lukemelia

Copy link
Copy Markdown
Contributor

[Claude Code 🤖] Adopted in e80d84e, in exactly this shape: the window resize listener and the trailing synchronous measure() are gone, and the ResizeObserver's initial post-commit callback is the sole first measurement — which is what keeps the tracked writes out of the render transaction that already read favoritesTileWidthPx. The one judgement call left as-is: the typeof ResizeObserver guard stays, since never-measured is a supported state (tilesPerRow falls back to 3 and the tile keeps its static CSS width), and the guard is where that contract is documented. The modifier's comment records both the initial-callback reliance and the resize-listener subsumption so neither gets reintroduced as a "fix".

ylm and others added 2 commits August 5, 2026 15:11
- The workspace chooser's menu footer now tracks a realm's updated_at within a
  session: on index completion the realm resource refreshes just its lifecycle
  timestamps from the federated-info batch (debounced, timestamp-only so it
  can't clobber the name or client-managed publish state, and a no-op for
  realms whose info isn't loaded or that live on another realm server). Adds
  RealmService#fetchRealmTimestamps for that fetch.
- Adds a realm-lifecycle test asserting a write to a source realm advances
  realm_registry.updated_at while created_at holds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Host tests failed with an uncaught "Cannot login to realm server without
  matrix client": the federated-info fetches added for mid-session timestamps
  (fetchInfoTask fallback) and the on-index timestamp refresh log in to the
  realm server, which throws when no matrix client is present (anonymous
  sessions and much of the test surface). Gate both on realmServer.hasClient,
  falling back to the token-authenticated per-realm /_info or a no-op.
- The new realm-lifecycle test sent backgroundURL/updatedAt as null (spread
  from testRealmInfo), which create-realm rejects with a 400 (present but not a
  string). Send string URLs like the sibling create-realm test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants